Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controls: Move into core. #29136

Merged
merged 1 commit into from
Aug 14, 2024
Merged

Controls: Move into core. #29136

merged 1 commit into from
Aug 14, 2024

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Aug 14, 2024

Related issue: #29079 (comment)

Description

As suggested in #29079 (comment), the PR moves Controls into core.

@Mugen87 Mugen87 added this to the r168 milestone Aug 14, 2024
Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
685.1 kB (169.6 kB) 685.4 kB (169.7 kB) +268 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
462 kB (111.4 kB) 462 kB (111.4 kB) +0 B

@Mugen87 Mugen87 merged commit bf6dfd5 into mrdoob:dev Aug 14, 2024
12 checks passed
@arodic
Copy link
Sponsor Contributor

arodic commented Aug 21, 2024

Wow!!! I’ve programmed my hibernation chamber to wake me up when this PR finally merges. I did not expect to wake up as soon as 2024!!!

Please forgive my snark but I do wonder what has changed?

I’ve made multiple efforts over the years to refactor controls that had a lot of duplicated logic and same bugs reappearing across multiple controls. The argument against it was that controls are just “examples” and don’t belong to threejs core. Has this stance finally changed???

@mrdoob
Copy link
Owner

mrdoob commented Aug 21, 2024

Only Controls.js has been added to core. The actual controls files remain in "examples" 😇

@arodic
Copy link
Sponsor Contributor

arodic commented Aug 21, 2024

Same thing I’ve proposed years ago. Anyway, super stoked this is finally happening!

@mrdoob
Copy link
Owner

mrdoob commented Aug 21, 2024

Slowly but surely!™ 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants